Skip to content

Conversation

@jchlanda
Copy link
Contributor

Make sure that a default value (nullptr) is returned from getIntrinsicParamType, also validate uses of this helper function.

Make sure that a default value (nullptr) is returned from
`getIntrinsicParamType`, also validate uses of this helper function.
@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Jakub Chlanda (jchlanda)

Changes

Make sure that a default value (nullptr) is returned from getIntrinsicParamType, also validate uses of this helper function.


Full diff: https://github.com/llvm/llvm-project/pull/122448.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp (+35-17)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp b/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
index 264b4d43248c69..09f2cad023e2e0 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp
@@ -929,23 +929,38 @@ AMDGPULibFuncBase::Param AMDGPULibFuncBase::Param::getFromTy(Type *Ty,
   return P;
 }
 
-static Type* getIntrinsicParamType(
-  LLVMContext& C,
-  const AMDGPULibFunc::Param& P,
-  bool useAddrSpace) {
-  Type* T = nullptr;
+static Type *getIntrinsicParamType(LLVMContext &C,
+                                   const AMDGPULibFunc::Param &P,
+                                   bool UseAddrSpace) {
+  Type *T = nullptr;
   switch (P.ArgType) {
+  default:
+    return nullptr;
   case AMDGPULibFunc::U8:
-  case AMDGPULibFunc::I8:   T = Type::getInt8Ty(C);   break;
+  case AMDGPULibFunc::I8:
+    T = Type::getInt8Ty(C);
+    break;
   case AMDGPULibFunc::U16:
-  case AMDGPULibFunc::I16:  T = Type::getInt16Ty(C);  break;
+  case AMDGPULibFunc::I16:
+    T = Type::getInt16Ty(C);
+    break;
   case AMDGPULibFunc::U32:
-  case AMDGPULibFunc::I32:  T = Type::getInt32Ty(C);  break;
+  case AMDGPULibFunc::I32:
+    T = Type::getInt32Ty(C);
+    break;
   case AMDGPULibFunc::U64:
-  case AMDGPULibFunc::I64:  T = Type::getInt64Ty(C);  break;
-  case AMDGPULibFunc::F16:  T = Type::getHalfTy(C);   break;
-  case AMDGPULibFunc::F32:  T = Type::getFloatTy(C);  break;
-  case AMDGPULibFunc::F64:  T = Type::getDoubleTy(C); break;
+  case AMDGPULibFunc::I64:
+    T = Type::getInt64Ty(C);
+    break;
+  case AMDGPULibFunc::F16:
+    T = Type::getHalfTy(C);
+    break;
+  case AMDGPULibFunc::F32:
+    T = Type::getFloatTy(C);
+    break;
+  case AMDGPULibFunc::F64:
+    T = Type::getDoubleTy(C);
+    break;
 
   case AMDGPULibFunc::IMG1DA:
   case AMDGPULibFunc::IMG1DB:
@@ -968,11 +983,11 @@ static Type* getIntrinsicParamType(
   case AMDGPULibFunc::DUMMY:
     return nullptr;
   }
-  if (P.VectorSize > 1)
+  if (T && P.VectorSize > 1)
     T = FixedVectorType::get(T, P.VectorSize);
   if (P.PtrKind != AMDGPULibFunc::BYVALUE)
     T = PointerType::get(
-        C, useAddrSpace ? ((P.PtrKind & AMDGPULibFunc::ADDR_SPACE) - 1) : 0);
+        C, UseAddrSpace ? ((P.PtrKind & AMDGPULibFunc::ADDR_SPACE) - 1) : 0);
   return T;
 }
 
@@ -989,9 +1004,11 @@ FunctionType *AMDGPUMangledLibFunc::getFunctionType(const Module &M) const {
     Args.push_back(ParamTy);
   }
 
-  return FunctionType::get(
-    getIntrinsicParamType(C, getRetType(FuncId, Leads), true),
-    Args, false);
+  Type *RetTy = getIntrinsicParamType(C, getRetType(FuncId, Leads), true);
+  if (!RetTy)
+    return nullptr;
+
+  return FunctionType::get(RetTy, Args, false);
 }
 
 unsigned AMDGPUMangledLibFunc::getNumArgs() const {
@@ -1080,6 +1097,7 @@ FunctionCallee AMDGPULibFunc::getOrInsertFunction(Module *M,
   }
 
   FunctionType *FuncTy = fInfo.getFunctionType(*M);
+  assert(FuncTy);
 
   bool hasPtr = false;
   for (FunctionType::param_iterator

@jchlanda
Copy link
Contributor Author

This was flagged by a downstream static analysis tool.

The diff of getIntrinsicParamType is slightly bigger, as it needed a clang format.

@jchlanda jchlanda merged commit c575a7d into llvm:main Jan 10, 2025
8 checks passed
BaiXilin pushed a commit to BaiXilin/llvm-project that referenced this pull request Jan 12, 2025
Make sure that a default value (nullptr) is returned from
`getIntrinsicParamType`, also validate uses of this helper function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants